Mahmoud/eng 3111 integrate with stacks raw hex signing on extension #652
Conversation
…ntegrate-with-stacks-raw-hex-signing-on-extension
…ntegrate-with-stacks-raw-hex-signing-on-extension
src/app/components/transactionsRequests/ContractCallRequest.tsx
Outdated
Show resolved
Hide resolved
|
nice work! 🙌 |
Conflicts: package-lock.json package.json src/app/screens/confirmFtTransaction/index.tsx src/app/screens/confirmStxTransaction/index.tsx src/app/screens/sendFt/index.tsx src/app/screens/signatureRequest/index.tsx src/app/screens/swap/useAlexSponsoredTransaction.ts src/app/screens/transactionRequest/helper.ts src/common/types/message-types.ts
src/inpage/sats.inpage.ts
Outdated
| @@ -30,7 +30,7 @@ const isValidEvent = (event: MessageEvent, method: SatsConnectMessageToContentSc | |||
| return correctSource && correctMethod && !!data.payload; | |||
| }; | |||
|
|
|||
| const SatsMethodsProvider: BitcoinProvider = { | |||
| const SatsMethodsProvider: Partial<BitcoinProvider> = { | |||
There was a problem hiding this comment.
I'm curious what was the reasoning for Partial here? 😄
do we want to hide type errors when we forget to add a new method from sats-connect? 🤔
There was a problem hiding this comment.
in my case, the version of sats-connect that we had in package.json was ahead of what was merged in develop at the time we might not want to keep it though, it's necessary now so the build doesn't fail
There was a problem hiding this comment.
ah I see. I'd be keen to remove it before merge, since it was a good code hint before
There was a problem hiding this comment.
@m-aboelenein did you have a chance to follow up on this one?
There was a problem hiding this comment.
nice catch i reverted that change 👍
| import { decodeToken } from 'jsontokens'; | ||
| import { useLocation } from 'react-router-dom'; | ||
|
|
||
| const useStxTransactionRequest = () => { |
There was a problem hiding this comment.
would be good to have a return type here. related to the core code review on txPayloadToRequest
There was a problem hiding this comment.
There's an issue with strictly typing this hook since the types that are exposed from the @stacks/connect lib have errors with the types themselves so until that's resolved loosely typing here is our only solution.
I will open an issue on the @stacks/connect repo and add a follow-up issue.
…ntegrate-with-stacks-raw-hex-signing-on-extension
…ntegrate-with-stacks-raw-hex-signing-on-extension
…ntegrate-with-stacks-raw-hex-signing-on-extension
| @@ -10,11 +10,10 @@ | |||
| "@ledgerhq/hw-transport-webusb": "^6.27.13", | |||
| "@phosphor-icons/react": "^2.0.10", | |||
| "@react-spring/web": "^9.6.1", | |||
| "@secretkeylabs/xverse-core": "6.0.1", | |||
| "@stacks/connect": "^6.10.2", | |||
| "@stacks/encryption": "4.3.5", | |||
There was a problem hiding this comment.
It looks like we currently still include @stacks/encryption in webpack/webpack.config.js, so I think it would make sense to remove it from there as well
DuskaT021
added
returned
|
Will be picked up for testing, after a comment gets resolved |
dhriaznov
added
ready for test
and removed
returned
|
m-aboelenein
deleted the
mahmoud/eng-3111-integrate-with-stacks-raw-hex-signing-on-extension
branch
🔘 PR Type
What kind of change does this PR introduce?
📜 Background
This PR introduces support for signing a transaction hex, which is parsed and displayed for the user for signing this enables stx multisig transactions
Issue Link: #ENG-3111
🔄 Changes
related core pr: secretkeylabs/xverse-core#285
✅ Review checklist
Please ensure the following are true before merging: